Refactored and prepared erlog for production use #13
Conversation
comtihon
commented
Jun 21, 2014
comtihon
commented
- got rid of passing functions everywhere. Maybe it is little bit faster, but code is hard to maintain. Refactored erlog gen_server, made erlog logic more OTP like: now there is one function to call - execute and all work is done through handle_call. Server state (db and others) is kept in #state of gen_server.
- refactored erlog_int, got rid of defining code (as from my point of view it is not good way), move storage functions from erlog_int to callback interface, made callback interface realisation for ets and dict (dict not working properly for now).
- created erlog_memory - gen_server, which works with memory (dict, ets, database, and others implementation). There is no need to pass db param as argument everywhere in code. It is stored is #state of erlog gen_server. (But I don't remove passing db in some places, as there are lots of them).
- moved all modules to folders
- moved functions from huge (500+ line of codes) modules to smaller one
- erlog_int -> erlog_memory, erlog_core
|
There is a lot of stuff here to go through! I will pull it locally and check it out. This may take some time as I am summer holidays and my programming has become more relaxed. Also I have to prepare a talk + demo for OSCON so please don't expect a speedy response, but I will get back to you on this. I noticed you have worked on the interface. I have just written up some independent thoughts on this to the erlog mailing list. It is a Google group at: https://groups.google.com/forum/?fromgroups=#!forum/erlog Please join if you wish. Traffic is low and I am trying to get more potential users and have it as a place for discussions. |
|
I joined and posted my answer. |
|
Question, why does execute return a binary take this code: fail_test() ->
{ok, PID} = erlog:start_link(),
?assertEqual(fail,erlog:execute(PID, "fail.")),
true.erlog:execute/2 returns the binary <<"No">>, might it make more sense to return it as an atom 'no' or better yet 'false'? It definitely seems more like other erlang functions to me |
|
It doesn't matter, I suppose, but if you prefer - it can be true/false. |
|
Well having it be bool() makes it work a lot better with many erlang things. |
|
I don't think it should return a boolean or a binary. I think proving goal should return the variable bindings if it succeeds so a boolean wouldn't fit, something like {true,Bindings} is just bad. I know we have a a few times in the libraries but that doesn't mean we should imitate it. And is shouldn't be false | NotBoolean. That is why I chose {succeed,Bindings} | fail. |
|
What @rvirding said, the return state should be what it was {succeed,[binding()]}|fail, also take a look at the quickcheck properties I created and lets see if we can make those work. |
|
So, what are the plans for merging? |
|
As your PR does some restructuring and renaming I would first like a discussion on how we want to structure the erlog distribution. I will start one on the mailing-list |
Conflicts: src/core/erlog.erl
